Be extra careful when locking and handle the case that the module may

2004-11-15  Matthias Clasen  <mclasen@redhat.com>

	* gdk-pixbuf-animation.c:
	* gdk-pixbuf-loader.c:
	* gdk-pixbuf-io.c: Be extra careful when locking and handle
	the case that the module may initialize the thread system.

	* gdk-pixbuf-io.[hc] (_gdk_pixbuf_lock): Return whether the
	lock was actually taken.

diff --git a/gdk-pixbuf/ChangeLog b/gdk-pixbuf/ChangeLog
index 786dd035b56b2140cf37ed91f6c26db91166ba58..454881041d68763d1643c4a136b820fd79626d11 100644 (file)
--- a/gdk-pixbuf/ChangeLog
+++ b/gdk-pixbuf/ChangeLog
@@ -1,5 +1,13 @@
 2004-11-15  Matthias Clasen  <mclasen@redhat.com>
 
+       * gdk-pixbuf-animation.c: 
+       * gdk-pixbuf-loader.c: 
+       * gdk-pixbuf-io.c: Be extra careful when locking and handle
+       the case that the module may initialize the thread system.
+
+       * gdk-pixbuf-io.[hc] (_gdk_pixbuf_lock): Return whether the
+       lock was actually taken.
+
        * queryloaders.c (query_module): Set vtable->module before
        calling fill_vtable(), since gdk-pixbuf-io.c does does the
        same and modules may rely on it.  (#158177, Dan Winship)

diff --git a/gdk-pixbuf/gdk-pixbuf-animation.c b/gdk-pixbuf/gdk-pixbuf-animation.c
index fa6968c153f75f08a7bf13c8a6171a3b6e03189a..56e5864e9292ff9625a4c90299a2baeb2a309516 100644 (file)
--- a/gdk-pixbuf/gdk-pixbuf-animation.c
+++ b/gdk-pixbuf/gdk-pixbuf-animation.c
@@ -137,6 +137,7 @@ gdk_pixbuf_animation_new_from_file (const char *filename,
        guchar buffer [128];
        GdkPixbufModule *image_module;
         gchar *display_name;
+        gboolean locked;
 
        g_return_val_if_fail (filename != NULL, NULL);
         g_return_val_if_fail (error == NULL || *error == NULL, NULL);
@@ -181,7 +182,7 @@ gdk_pixbuf_animation_new_from_file (const char *filename,
                         return NULL;
                 }
 
-        _gdk_pixbuf_lock (image_module);
+        locked = _gdk_pixbuf_lock (image_module);
 
        if (image_module->load_animation == NULL) {
                GdkPixbuf *pixbuf;
@@ -245,7 +246,8 @@ gdk_pixbuf_animation_new_from_file (const char *filename,
         g_free (display_name);
 
  out_unlock:
-        _gdk_pixbuf_unlock (image_module);
+        if (locked)
+                _gdk_pixbuf_unlock (image_module);
        return animation;
 }
 

diff --git a/gdk-pixbuf/gdk-pixbuf-io.c b/gdk-pixbuf/gdk-pixbuf-io.c
index 5c2157e3eab6df5f278d9eee63faefa59a32e325..02d4eed615e4e8e128855a17a7179bb1fae35cd5 100644 (file)
--- a/gdk-pixbuf/gdk-pixbuf-io.c
+++ b/gdk-pixbuf/gdk-pixbuf-io.c
@@ -79,12 +79,17 @@ format_check (GdkPixbufModule *module, guchar *buffer, int size)
 G_LOCK_DEFINE_STATIC (init_lock);
 G_LOCK_DEFINE_STATIC (threadunsafe_loader_lock);
 
-void
+gboolean
 _gdk_pixbuf_lock (GdkPixbufModule *image_module)
 {
-       if (!(image_module->info->flags & GDK_PIXBUF_FORMAT_THREADSAFE)) {
+       if (g_threads_got_initialized &&
+           !(image_module->info->flags & GDK_PIXBUF_FORMAT_THREADSAFE)) {
                G_LOCK (threadunsafe_loader_lock);
+
+               return TRUE;
        }
+
+       return FALSE;
 }
  
 void
@@ -460,10 +465,19 @@ _gdk_pixbuf_load_module (GdkPixbufModule *image_module,
                         GError         **error)
 {
        gboolean ret;
+       gboolean locked = FALSE;
 
-       G_LOCK (init_lock);
+       /* be extra careful, maybe the module initializes
+        * the thread system
+        */
+       if (g_threads_got_initialized)
+       {
+               G_LOCK (init_lock);
+               locked = TRUE;
+       }
        ret = _gdk_pixbuf_load_module_unlocked (image_module, error);
-       G_UNLOCK (init_lock);
+       if (locked)
+               G_UNLOCK (init_lock);
        return ret;
 }
 
@@ -736,8 +750,9 @@ _gdk_pixbuf_generic_image_load (GdkPixbufModule *module,
        GdkPixbuf *pixbuf = NULL;
        GdkPixbufAnimation *animation = NULL;
        gpointer context;
+       gboolean locked;
 
-       _gdk_pixbuf_lock (module);
+       locked = _gdk_pixbuf_lock (module);
 
        if (module->load != NULL) {
                pixbuf = (* module->load) (f, error);
@@ -773,13 +788,13 @@ _gdk_pixbuf_generic_image_load (GdkPixbufModule *module,
                        pixbuf = gdk_pixbuf_animation_get_static_image (animation);
 
                        g_object_ref (pixbuf);
-
                        g_object_unref (animation);
                }
        }
 
  out:
-       _gdk_pixbuf_unlock (module);
+       if (locked)
+               _gdk_pixbuf_unlock (module);
        return pixbuf;
 }
 
@@ -1154,7 +1169,10 @@ gdk_pixbuf_new_from_xpm_data (const char **data)
        GdkPixbuf *(* load_xpm_data) (const char **data);
        GdkPixbuf *pixbuf;
         GError *error = NULL;
-       GdkPixbufModule *xpm_module = _gdk_pixbuf_get_named_module ("xpm", &error);
+       GdkPixbufModule *xpm_module;
+       gboolean locked;
+
+       xpm_module = _gdk_pixbuf_get_named_module ("xpm", &error);
        if (xpm_module == NULL) {
                g_warning ("Error loading XPM image loader: %s", error->message);
                g_error_free (error);
@@ -1169,7 +1187,7 @@ gdk_pixbuf_new_from_xpm_data (const char **data)
                 }
         }
 
-       _gdk_pixbuf_lock (xpm_module);
+       locked = _gdk_pixbuf_lock (xpm_module);
 
        if (xpm_module->load_xpm_data == NULL) {
                g_warning ("gdk-pixbuf XPM module lacks XPM data capability");
@@ -1178,8 +1196,9 @@ gdk_pixbuf_new_from_xpm_data (const char **data)
                load_xpm_data = xpm_module->load_xpm_data;
                pixbuf = (* load_xpm_data) (data);
        }
-
-       _gdk_pixbuf_unlock (xpm_module);
+       
+       if (locked)
+               _gdk_pixbuf_unlock (xpm_module);
        return pixbuf;
 }
 
@@ -1250,6 +1269,7 @@ gdk_pixbuf_real_save (GdkPixbuf     *pixbuf,
 {
        gboolean ret;
        GdkPixbufModule *image_module = NULL;       
+       gboolean locked;
 
        image_module = _gdk_pixbuf_get_named_module (type, error);
 
@@ -1260,7 +1280,7 @@ gdk_pixbuf_real_save (GdkPixbuf     *pixbuf,
                if (!_gdk_pixbuf_load_module (image_module, error))
                        return FALSE;
 
-       _gdk_pixbuf_lock (image_module);
+       locked = _gdk_pixbuf_lock (image_module);
 
        if (image_module->save) {
                /* save normally */
@@ -1283,7 +1303,8 @@ gdk_pixbuf_real_save (GdkPixbuf     *pixbuf,
                ret = FALSE;
        }
 
-       _gdk_pixbuf_unlock (image_module);
+       if (locked)
+               _gdk_pixbuf_unlock (image_module);
        return ret;
 }
 
@@ -1304,6 +1325,7 @@ save_to_callback_with_tmp_file (GdkPixbufModule   *image_module,
        gchar *buf = NULL;
        gsize n;
        gchar *filename = NULL;
+       gboolean locked;
 
        buf = g_try_malloc (TMP_FILE_BUF_SIZE);
        if (buf == NULL) {
@@ -1326,9 +1348,10 @@ save_to_callback_with_tmp_file (GdkPixbufModule   *image_module,
                goto end;
        }
 
-       _gdk_pixbuf_lock (image_module);
+       locked = _gdk_pixbuf_lock (image_module);
        retval = (image_module->save) (f, pixbuf, keys, values, error);
-       _gdk_pixbuf_unlock (image_module);
+       if (locked)
+               _gdk_pixbuf_unlock (image_module);
        if (!retval)
                goto end;
 
@@ -1375,6 +1398,7 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf         *pixbuf,
 {
        gboolean ret;
        GdkPixbufModule *image_module = NULL;       
+       gboolean locked;
 
        image_module = _gdk_pixbuf_get_named_module (type, error);
 
@@ -1385,7 +1409,7 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf         *pixbuf,
                if (!_gdk_pixbuf_load_module (image_module, error))
                        return FALSE;
 
-       _gdk_pixbuf_lock (image_module);
+       locked = _gdk_pixbuf_lock (image_module);
 
        if (image_module->save_to_callback) {
                /* save normally */
@@ -1408,7 +1432,8 @@ gdk_pixbuf_real_save_to_callback (GdkPixbuf         *pixbuf,
                ret = FALSE;
        }
 
-       _gdk_pixbuf_unlock (image_module);
+       if (locked)
+               _gdk_pixbuf_unlock (image_module);
        return ret;
 }
 

diff --git a/gdk-pixbuf/gdk-pixbuf-loader.c b/gdk-pixbuf/gdk-pixbuf-loader.c
index 0ce63605d31393a5f64c49d5566fb0ae00391082..9e252c5706213b99fe9bf72370475d7aaa7b00f2 100644 (file)
--- a/gdk-pixbuf/gdk-pixbuf-loader.c
+++ b/gdk-pixbuf/gdk-pixbuf-loader.c
@@ -387,8 +387,7 @@ gdk_pixbuf_loader_load_module (GdkPixbufLoader *loader,
                 }
 
        if (!priv->holds_threadlock) {
-                _gdk_pixbuf_lock (priv->image_module);
-                priv->holds_threadlock = TRUE;
+                priv->holds_threadlock = _gdk_pixbuf_lock (priv->image_module);
         }
 
         priv->context = priv->image_module->begin_load (gdk_pixbuf_loader_size_func,
@@ -745,8 +744,7 @@ gdk_pixbuf_loader_close (GdkPixbufLoader *loader,
                 }
   
         priv->closed = TRUE;
-       if (priv->image_module) {
-                g_assert (priv->holds_threadlock);
+       if (priv->image_module && priv->holds_threadlock) {
                 _gdk_pixbuf_unlock (priv->image_module);
                 priv->holds_threadlock = FALSE;
         }